removed need for friend declaration of test class in CmdLineParser / cleanups
#7896
Conversation
|
|
If this is acceptable we probably can use this pattern for other classes which currently require |
| class CmdLineParserTest : public CmdLineParser | ||
| { | ||
| friend class TestCmdlineParser; |
There was a problem hiding this comment.
This allows us to access the protected members.
There was a problem hiding this comment.
I see no nice solution.
- Friend is horrible but at least it explicitly pointed out the test class.
- The disadvantage with this solution is that if we would ever like to use CmdLineParser as a base class in production code then the derived classes have access to stuff that should really be private.
I have no strong opinion. But I fear that sooner or later somebody will come and look at the code and in his opinion we should have solved it in another way and then we get another PR..
I think it is perfectly reasonable in tests. It might even be the proper way to test internals.
Which you could always circumvent by using a Also this allowed us to remove several unnecessary public accesses so you can actually access less things then before from outside the class. In some other cases I already did we can even get rid of the Using the |
|
I could have kept the access functions and moved them to the test classes. Something to consider for future changes like this. |
2 participants
No description provided.